Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a265c73) 48.87% compared to head (d9497d6) 48.87%.

@@           Coverage Diff           @@
##           master    #2413   +/-   ##
=======================================
  Coverage   48.87%   48.87%           
=======================================
  Files         124      124           
  Lines        5238     5238           
  Branches     1109     1109           
=======================================
  Hits         2560     2560           
  Misses       2371     2371           
  Partials      307      307           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Hmmm I see.. this current implementation adds line numbers regardless of user's settings.
I'm concerned about whether this would be flexible enough for the user
What do @kaixin-hc @EltonGohJH @itsyme think?

Hmmm I see.. this current implementation adds line numbers regardless of user's settings. I'm concerned about whether this would be flexible enough for the user What do @kaixin-hc @EltonGohJH @itsyme think?

I agreed that it will be inflexible and the users may not expect this behaviour. Maybe we can target the printing aspect?

Wdyt?

@yucheng11122017 @kaixin-hc @itsyme

It feels like the main issue is what if it wraps line numbers the user does not expect to wrap (since not all languages consider indents the same way as python). If we force this change for all wrapping when clicked regardless of users settings, I'm not sure yet that there would be no undesired behaviour.

If when printing, not wrapping and wrapping but no line numbers both often distort the meaning unexpectedly ... Then I agree with Elton that a change made only for print makes sense? In other words when printing code blocks will auto wrap and add line numbers, otherwise it will follow existing behaviours.

Another alternative (but breaking) might be to have all code blocks automatically have line numbers that must be manually disabled, which would cause the default behaviour to be "correct" when printing if not otherwise overriden. But it's probably not worth making a breaking change for this.

The current behavior would definitely be a breaking change as well. I would prefer to add only when printing as well
@damithc is that ok?

If when printing, not wrapping and wrapping but no line numbers both often distort the meaning unexpectedly ... Then I agree with Elton that a change made only for print makes sense? In other words when printing code blocks will auto wrap and add line numbers, otherwise it will follow existing behaviours.

Yes, when printing, should be auto-wrapped (but only if there are lines longer than the container width), and line numbers should be added. However, if wrapping is not needed for a given code block, the current printing behavior should remain (i.e., no line numbers unless already enabled by the user), right?

Another alternative (but breaking) might be to have all code blocks automatically have line numbers that must be manually disabled, which would cause the default behaviour to be "correct" when printing if not otherwise overriden. But it's probably not worth making a breaking change for this.

This is what we had in an earlier version but we changed it because we found most of the time code blocks don't need line numbers and it was a hassle to disable it in many places. So, we decided the default to be 'no line numbers'.

It feels like the main issue is what if it wraps line numbers the user does not expect to wrap (since not all languages consider indents the same way as python). If we force this change for all wrapping when clicked regardless of users settings, I'm not sure yet that there would be no undesired behaviour.

The current behavior of the line-wrapping plugin is already flawed because it doesn't differentiate between real line breaks and soft/fake line breaks. Alternatives:

1. Add a new symbol to indicate soft line breaks, like Intellij does it: This seems hard to do, and requires the reader to know the meaning of the new symbol.

2. Show line numbers (even if not enabled) when line wrapping: This seems easier to do, and more intuitive for the reader to understand. It may be unexpected for the reader to see line numbers, but line numbers are harmless, so should be fine?

Thanks for the additional context prof! Found #1671 where this is discussed and #1734 .

TLDR: agree about softwrapping with line numbers for overflowing code on print (but no change for non overflowing code) makes sense because it helps to avoid changing/losing the meaning of the code, which is the worst part in thr status quo.

Additional rambles on alternative 2:

Alternative 2, which is what I believe this PR is suggesting is followed on several websites. GitHub has line numbers and softwraps all code on mobile; but in the MarkBind context this might not make sense:

Wrapped (Github)	Not wrapped (MarkBind)

Admittedly, in cases like that (an ASCII image that overflows the page size) whether it softwraps on printing or not it will still look weird, and is a rare edge case.

After doing some reading, it seems no line numbers is (frequently) preferred from a UX perspective(discussion 1 (ux exchange), discussion 2 (reddit)), but individuals often have the opposite preference...

So I'm hesitant to say all code wrapped blocks should enable line numbers. If you wrap the code and the meaning doesn't change when you think it's a hard line break instead of a soft line break, I can see why a user who doesn't like line numbers would still want line numbers to be disabled even with code wrapped. Having the numbers go back on would be unexpected (and breaking?)

Hmmm I see.. this current implementation adds line numbers regardless of user's settings. I'm concerned about whether this would be flexible enough for the user What do @kaixin-hc @EltonGohJH @itsyme think?

I agreed that it will be inflexible and the users may not expect this behaviour. Maybe we can target the printing aspect?

Wdyt?

@yucheng11122017 @kaixin-hc @itsyme

I agree with @EltonGohJH on targeting only the printing aspect as that is the only situation where the user is unable to scroll and the line numbers will improve the clarity in this situation.

Yes, when printing, should be auto-wrapped (but only if there are lines longer than the container width), and line numbers should be added. However, if wrapping is not needed for a given code block, the current printing behavior should remain (i.e., no line numbers unless already enabled by the user), right?

I agree with this behaviour as well

Hmmm I see.. this current implementation adds line numbers regardless of user's settings. I'm concerned about whether this would be flexible enough for the user What do @kaixin-hc @EltonGohJH @itsyme think?

I agreed that it will be inflexible and the users may not expect this behaviour. Maybe we can target the printing aspect?
Wdyt?
@yucheng11122017 @kaixin-hc @itsyme

I agree with @EltonGohJH on targeting only the printing aspect as that is the only situation where the user is unable to scroll and the line numbers will improve the clarity in this situation.

Yes, when printing, should be auto-wrapped (but only if there are lines longer than the container width), and line numbers should be added. However, if wrapping is not needed for a given code block, the current printing behavior should remain (i.e., no line numbers unless already enabled by the user), right?

I agree with this behaviour as well
Sounds great!

I cannot think of a way to determine if wrapping happens. Currently, line-numbers are added in markdown-it. Wrapping seems to be handled by css and whether it actually happens is hard to tell. Can anyone suggest any solution to this?

I cannot think of a way to determine if wrapping happens. Currently, line-numbers are added in markdown-it. Wrapping seems to be handled by css and whether it actually happens is hard to tell. Can anyone suggest any solution to this?

Hi @Tim-Siu if im understanding this correctly, the idea is that line numbers will naturally show if there is line wrapping already? Because if there is wrapping, a new line will be present but the numbers doesn't increase. Please correct me if i'm understanding your question wrongly! :)

I cannot think of a way to determine if wrapping happens. Currently, line-numbers are added in markdown-it. Wrapping seems to be handled by css and whether it actually happens is hard to tell. Can anyone suggest any solution to this?

Hi @Tim-Siu if im understanding this correctly, the idea is that line numbers will naturally show if there is line wrapping already? Because if there is wrapping, a new line will be present but the numbers doesn't increase. Please correct me if i'm understanding your question wrongly! :)

Hi @Tim-Siu, is there anything you want to clarify? Is there any problem you are encountering?

I cannot think of a way to determine if wrapping happens. Currently, line-numbers are added in markdown-it. Wrapping seems to be handled by css and whether it actually happens is hard to tell. Can anyone suggest any solution to this?

Hi @Tim-Siu if im understanding this correctly, the idea is that line numbers will naturally show if there is line wrapping already? Because if there is wrapping, a new line will be present but the numbers doesn't increase. Please correct me if i'm understanding your question wrongly! :)

Hi @Tim-Siu, is there anything you want to clarify? Is there any problem you are encountering?

Yes, you understanding of the current implementation in the PR is correct. I haven't figured out ways to implement the suggestions though. I am still investigating.

Hi @Tim-Siu is this PR still relevant?

Hi @Tim-Siu is this PR still relevant?

Hi @yucheng11122017 it can be closed if we've decided to only target the printing aspect.

Closed as no longer relevant - replaced by #2431